fix: add buffer-length check in unzip.cpp by anupamme · Pull Request #429 · NativeScript/ios

anupamme · 2026-08-04T00:20:20Z

Summary

Fix critical severity security issue in TKLiveSync/unzip.cpp.

Vulnerability

Field	Value
ID	V-001
Severity	CRITICAL
Scanner	multi_agent_ai
Rule	`V-001`
File	`TKLiveSync/unzip.cpp:49`
Assessment	Likely exploitable
CWE	CWE-120

Description: A PATH_MAX-sized heap buffer (pathcopy) receives ZIP entry names via strcpy() without bounds checking. ZIP specification allows entry names up to 65535 bytes, far exceeding typical PATH_MAX values (4096 or 1024). This creates a classic buffer overflow where crafted long filenames overflow the heap buffer.

Evidence

Exploitation scenario: Attacker creates a ZIP archive with an entry name longer than PATH_MAX bytes.

Scanner confirmation: multi_agent_ai rule V-001 flagged this pattern.

Production code: This file is in the production codebase, not test-only code.

Threat Model Context

This is a Node.js library - vulnerabilities affect downstream consumers who use this package.

Changes

TKLiveSync/unzip.cpp

Behavior Preservation

The change is scoped to 1 file on the vulnerable path; it only tightens handling of untrusted input and leaves valid inputs unaffected.

Automated security fix by OrbisAI Security

Summary by CodeRabbit

Bug Fixes
- Improved ZIP extraction reliability by rejecting empty, absolute, or unsafe entry paths.
- Handled directory paths more safely during extraction.
- Avoided unnecessary directory creation when entries do not include a parent directory.
Refactor
- Simplified archive path handling for improved stability and maintainability.

Automated security fix generated by OrbisAI Security

coderabbitai · 2026-08-04T00:20:33Z

📝 Walkthrough

Walkthrough

The ZIP extraction code validates entry names before processing. It rejects unsafe paths and extracts parent directories with std::string without using a temporary buffer.

Changes

ZIP extraction safety and directory handling

Layer / File(s)	Summary
ZIP entry-name validation `TKLiveSync/unzip.cpp`	The code skips empty, absolute, and `..`-containing entry names before it constructs output paths or opens files.
Parent directory extraction and cleanup `TKLiveSync/unzip.cpp`	The code removes the unused `<libgen.h>` dependency and `pathcopy` allocation. It creates parent directories only when an entry contains a slash.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

A rabbit checks each ZIP path,
And blocks unsafe tracks.
Parent folders use strings anew,
No spare buffer hops through.
Safe names open the door,
Clean paths reach the floor.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary fix for the buffer overflow in unzip.cpp.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

NathanWalker · 2026-08-04T05:50:41Z

Thank you, target main branch instead of dev on this PR.

anupamme · 2026-08-04T06:01:29Z

Thank you, target main branch instead of dev on this PR.

done.

Replace heap-allocated PATH_MAX buffer + strcpy/dirname with std::string find_last_of to avoid silent truncation of long ZIP entry names that could cause the directory path to diverge from assetFullname. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@TKLiveSync/unzip.cpp`:
- Around line 47-55: Validate each archive entry name in unzip() before
constructing directory paths or opening files: reject absolute paths and any
path component exactly equal to "..". Treat invalid names as failed entries and
skip further processing, ensuring mkdir_rec() and fopen() are never called for
them.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 441a7f98-d430-4747-bfef-2104aa141350

📥 Commits

Reviewing files that changed from the base of the PR and between 77b1ea3 and da985c2.

📒 Files selected for processing (1)

TKLiveSync/unzip.cpp

Entry names from sync.zip are untrusted, yet unzip() appended them verbatim to the destination before mkdir/fopen. Names containing ".." components or absolute paths escaped the LiveSync directory, letting a crafted archive create or overwrite files outside it (ZipSlip). Validate each entry with is_safe_entry_name() and skip entries that are absolute or contain a ".." path component. Legitimate archives extract unchanged.

coderabbitai

🧹 Nitpick comments (1)

TKLiveSync/unzip.cpp (1)
28-47: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add regression tests for the path-validation boundary.

Cover nested safe names, absolute names, .. components, empty names, and names longer than PATH_MAX. Run the long-name case under AddressSanitizer to protect the heap-overflow fix from regression.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@TKLiveSync/unzip.cpp` around lines 28 - 47, Add regression tests targeting
is_safe_entry_name for nested safe paths, absolute paths, parent-directory
components, empty names, and names exceeding PATH_MAX. Verify each boundary case
is accepted or rejected as intended, and execute the overlong-name test with
AddressSanitizer enabled to detect heap overflows.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@TKLiveSync/unzip.cpp`:
- Around line 28-47: Add regression tests targeting is_safe_entry_name for
nested safe paths, absolute paths, parent-directory components, empty names, and
names exceeding PATH_MAX. Verify each boundary case is accepted or rejected as
intended, and execute the overlong-name test with AddressSanitizer enabled to
detect heap overflows.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 76e5ed03-cd5b-44ba-8d2e-a51171124d75

📥 Commits

Reviewing files that changed from the base of the PR and between da985c2 and e473cef.

📒 Files selected for processing (1)

TKLiveSync/unzip.cpp

fix: V-001 security vulnerability

a0ae972

Automated security fix generated by OrbisAI Security

anupamme changed the base branch from dev to main August 4, 2026 06:01

NathanWalker self-assigned this Aug 6, 2026

NathanWalker added the in progress label Aug 6, 2026

NathanWalker requested changes Aug 6, 2026

View reviewed changes

Comment thread TKLiveSync/unzip.cpp Outdated

coderabbitai Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread TKLiveSync/unzip.cpp

NathanWalker approved these changes Aug 8, 2026

View reviewed changes

coderabbitai Bot reviewed Aug 8, 2026

View reviewed changes

NathanWalker merged commit 131611a into NativeScript:main Aug 8, 2026
4 checks passed

Navigation Menu

Uh oh!

Uh oh!

fix: add buffer-length check in unzip.cpp - #429

fix: add buffer-length check in unzip.cpp#429
NathanWalker merged 3 commits into
NativeScript:mainfrom
anupamme:fix-repo-ios-heap-buffer-overflow-unzip-strcpy

anupamme commented Aug 4, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

NathanWalker commented Aug 4, 2026

Uh oh!

anupamme commented Aug 4, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

anupamme commented Aug 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Vulnerability

Evidence

Threat Model Context

Changes

Behavior Preservation

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

NathanWalker commented Aug 4, 2026

Uh oh!

anupamme commented Aug 4, 2026

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anupamme commented Aug 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 4, 2026 •

edited

Loading